Skip to content

Tests: adapt HTML escaping to pygments v2.21.0 change-in-behaviour - #14611

Merged
picnixz merged 3 commits into
sphinx-doc:masterfrom
jayaddison:issue-14610/ci-tests-html-escaping-fixup
Sep 18, 2026
Merged

picnixz merged 3 commits into
sphinx-doc:masterfrom
jayaddison:issue-14610/ci-tests-html-escaping-fixup

Conversation

@jayaddison

@jayaddison jayaddison commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

This pull request should allow the Sphinx Python unit test suite to pass; it began failing recently following the release of Pygments v2.21.0 that included adjustments to HTML quote-escaping logic: pygments/pygments#3185

This changeset adapts the relevant unit test expectations according to the runtime version of Pygments detected.

References

AI Disclosure

I did not use any AI/LLM tooling for this pull request; neither to develop the changes locally, nor to create the pull request itself.


Edit: correction: a different Pygments PR seems to have been the cause of this; the hyperlink has been updated
Edit: rephrase for conciseness/clarity

TODO: determine why this is necessary; what has changed about
the HTML escaping recently that requires this?
@jayaddison

This comment was marked as resolved.

@jayaddison jayaddison changed the title Tests: reduce HTML escaping to allow tests to pass Tests: adapt HTML escaping to pygments v2.21.0 change-in-behaviour Aug 17, 2026
@jayaddison
jayaddison marked this pull request as ready for review August 17, 2026 11:29

@jdillard jdillard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracking this down. This looks like the right approach as these tests already branch on the Pygments 2.19 whitespace-span change, so dual expectations here make sense.

Just FYI, I'm pretty sure the visible output change came from pygments#3185 (look at the note at the bottom of the description).

Comment thread tests/test_highlighting.py Resolved
@jdillard jdillard added this to the 9.1.1 milestone Aug 18, 2026
@jayaddison

Copy link
Copy Markdown
Contributor Author

@jdillard

Just FYI, I'm pretty sure the visible output change came from pygments#3185 (look at the note at the bottom of the description).

Thanks - I've updated the PR description to link to that instead.

@jdillard jdillard mentioned this pull request Aug 18, 2026
@jdillard

Copy link
Copy Markdown
Member

@picnixz a new CI test failure just dropped (you can see how it manifests in #14241 for example). It would be great to get this one merged, when you get a chance, before it starts to complicate PRs

@jayaddison

Copy link
Copy Markdown
Contributor Author

@picnixz a small nudge/reminder in case this can be merged

@shashb27

shashb27 commented Sep 2, 2026

Copy link
Copy Markdown

Independent verification, in case it helps the review: on Windows (Python 3.13.13, Pygments 2.21.0), the currently-red set — test_viewcode, test_viewcode_linenos, test_default_highlight, test_html_code_role, and both test_additional_targets_* intl tests — fails on master-equivalent code and passes on this branch (commit bb31616). With Pygments 2.20.0 the same six pass on both, consistent with the green Oldest supported job, so the version-conditional expectations hold on both sides of the change.

For context: every open PR's test matrix is currently red on exactly this set (the runs on #14654, #14662, and #14666 all show the same six failures), so merging this would restore review signal across the board. cc @picnixz

@shashb27

Copy link
Copy Markdown

Gentle follow-up: this has been approved for a month now, and the Pygments 2.21 failures it fixes are still red across much of the open PR queue, including the viewcode PRs and our #14666. The Windows verification across Pygments 2.20 and 2.21 from early September still holds on current master. I also see #14674 covers part of the same ground. No preference from me on which route lands, but getting one of them in would unblock a lot of CI. Is there anything holding it up that an outside contributor could help with?

@picnixz

picnixz commented Sep 18, 2026

Copy link
Copy Markdown
Member

Ah sorry, I'm just overwhelmed by my daily job and travels that I don't have enough time for OSS. One comment though: can you refactor the pygments version split and add some PYGMENTS_VERSION_INFO = tuple(...) in some util module that we could import from everywhere in the tests instead of having to recompute the version info tuple in every test? for now I'm going to merge this but please make a follow-up if you can find the time.

@picnixz
picnixz merged commit b04a210 into sphinx-doc:master Sep 18, 2026
28 checks passed
@jayaddison

Copy link
Copy Markdown
Contributor Author

Thanks @picnixz - yep, I'll take a look into refactoring that at some point (I can't promise when, I'm finding it hard to find time to focus here at the moment too). I'll also compare to how any other library/version checks are performed in the test suite.

@jayaddison
jayaddison deleted the issue-14610/ci-tests-html-escaping-fixup branch September 18, 2026 09:49
@picnixz

picnixz commented Sep 18, 2026

Copy link
Copy Markdown
Member

I see that docutils and latex are still failing. Is there a PR I can merge for that?

@jayaddison

Copy link
Copy Markdown
Contributor Author

@picnixz

I see that docutils and latex are still failing. Is there a PR I can merge for that?

Pull request #14673 from @adamtheturtle resolves most of the docutils HEAD issues.

⚠️ However...

When I run that locally today (using the docutils mirror on GitHub -- because repo.or.cz is geoblocked from the United Kingdom) the remaining failures are:

  1. HTML output tests where RFC URLs, instead of emitting .html suffixes, emit empty-directory (trailing slash) URLs:

(
'markup.html',
".//a[@href='https://datatracker.ietf.org/doc/html/rfc1.html']"
"[@class='rfc reference external']/strong",
'RFC 1',
),
(
'markup.html',
".//a[@href='https://datatracker.ietf.org/doc/html/rfc1.html']"
"[@class='rfc reference external']/strong",
'Request for Comments #1',
),

  1. A failure in test_cfunction where the following diff occurs:
E         Full diff:
E           [
E               '',
E         -     '.. py:function:: asctime([tuple]) -> string',
E         +     '.. py:function:: asctime([time_tuple]) -> string',
E         ?                                 +++++
E               '   :module: time',
E               '',
E               "   Convert a time tuple to a string, e.g. 'Sat Jun 06 16:26:11 1998'.",
E               '   When the time tuple is not present, current time as returned by '
E               'localtime()',
E               '   is used.',
E               '',
E           ]

tests/test_ext_autodoc/test_ext_autodoc_autofunction.py:126: AssertionError

@adamtheturtle

Copy link
Copy Markdown
Contributor

@jayaddison FWIW I don't have time right now to contribute further to that, but I take no offence at someone taking my work and getting it to shippable!

@jayaddison

Copy link
Copy Markdown
Contributor Author

Ok, thanks @adamtheturtle. Please could you rebase and/or merge-from-master into the branch so that the latest CI status is more clearly visible?

@jayaddison

Copy link
Copy Markdown
Contributor Author

Thanks!

@jayaddison

jayaddison commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor Author

Self-quoting:

2. A failure in [`test_cfunction`](https://github.com/sphinx-doc/sphinx/blob/b04a2101295ac3fb725b16111eda0284b6da4cca/tests/test_ext_autodoc/test_ext_autodoc_autofunction.py#L124-L135) where the following diff occurs:

Note: that failure has been reported resolved already in #14683 (thanks @mitya); it's due to the fact that python3.14 in Debian testing (that I ran the tests with) includes python/cpython#155790

I'll try to take a look into resolving that soon too (although it may be tricky to use a Python system version as a range boundary, since no formal tagged release of cPython yet includes that change..)

Edit: thread #14683 is a pull request, not an issue report thread as I initially thought. It includes a fix, and the fix looks good to me.

@Darlokt

Darlokt commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Hej, @picnixz the Docutils HEAD failures are part of currently the Docutils 1.0 breaking changes being merged on dev one by one, so even with current fixes, the failures will continue, until the possible interceptions have landed and/or are resolved.
I have been trying to open issues one by one as they are merged, but for many we have to wait for the upstream merge to see the final implementation to test against.
In #14680 I made this small table of what I could find in the Docutils changes that affect Sphinx (In line with the AI Policy: I used the help of AI to aggregate all the open already issues and PRs and summarize upstream changes) (Also, the last one is a catch all, many of the changes dont affect sphinx itself (or shouldn't according to what I can see) but could add-ons, as there are some node changes etc.)
I added the PR by @jayaddison to the table (#14696 provides a partial fix to #14680 and doesnt fix #14659, adjusting tests to pass for now.) (@jayaddison for the RFC changes I suggest to decouple partially from Docutils, as the current construction seems to be an artifact from development to make the construction version agnostic across versions as the new notation is the correct one going forward and should replace the existing one. For the table changes we have to be a bit more careful with cross version behavior with fractional/proportional widths (which I think will be bigger piece of writer work for Sphinx going towards Docutils 2.0) and indented doctests, also to not make current edge behavior programmatically dependent. If you want you can look at my PRs #14681 and #14689 how I suggest to fix them.):

Docutils change Upstream status Effect on Sphinx Sphinx issue Sphinx PRs Review and merge status
Native doctests changed from doctest_block to classified literal_block nodes; table colwidth values changed from integers to strings Landed Breaks doctest highlighting and collection, movement of indented doctests, autosummary XML output, and table rendering in the text, LaTeX, and Texinfo writers #14680, open #14681, #14696 (contains AI #14673) partial, (AI: #14685) None approved or merged
RFC base changed to https://www.rfc-editor.org/info/, and generated RFC paths no longer end in .html Landed Sphinx defines its own base URL but uses Docutils' RFC path template. This produces changed URLs and four failing assertions #14659, open #14689, #14662, #14696 (test accommodation only) None approved or merged
DanglingReferences split into MatchReferences, ReportDanglingReferences, and ReportUnreferencedTargets Landed Sphinx removes only the old aggregate transform and adds its own subclass of it. The new transforms therefore remain active alongside Sphinx's old transform. Docutils documents ReportUnreferencedTargets as unsuitable for Sphinx's multi-document model None None Unaddressed , Cant be addressed really as of now, as the depreciation warning appears in 1.0 but the replacements aren't available until 1.0. Would need feature dependent handling and lifecycle testing.
Figures without a caption or legend now produce a warning Landed Broke warning-as-error testing because two Sphinx test fixtures contained invalid figures #14508 closed #14540 #14540 was approved and merged.
Move HTML5 attribution markup after its blockquote Announced for 1.0, not landed Sphinx overrides blockquote rendering to add an inner <div> but inherits Docutils' attribution handling. The upstream change may alter Sphinx's HTML structure or conflict with its wrapper markup and themes None None Awaiting upstream implementation and Sphinx regression coverage
Change the manpage text_references default to False Announced for 1.0, not landed Docutils installs an instance-level reference visitor based on this setting, shadowing Sphinx's visit_reference. The new default will select different Docutils rendering unless Sphinx controls dispatch itself #14427, open #14436 None approved or merged. It likely covers the future default change but must be retested after that change lands
Use SOURCE_DATE_EPOCH for the Docutils date directive and datestamp setting Announced for 1.0, not landed Sphinx uses the Docutils directive and already honors SOURCE_DATE_EPOCH elsewhere. The change is likely desirable but needs a Sphinx regression test None None Awaiting upstream implementation and Sphinx regression coverage
Other document-tree and API changes relevant to third-party extensions Some landed Sphinx core has no reproduced failure. Extensions may be affected if they expect eager IDs, inspect old inline-target or doctest nodes, assume integer column widths, subclass DanglingReferences, or import removed Docutils APIs None None Not a Sphinx-core blocker without a reproducer. Relevant migration notes may be needed before enabling Docutils 1.0

As this will continue, the question will be whether for now Docutils HEAD tests should be paused and/or if we want to open a tracking issue, as there is a bit of work open that has been announced in the Docutils Release Notes that will land in the coming days to weeks.

Disclaimer, I also wrote the PRs #14681, #14689.

@saitanallensantiago26-beep

Copy link
Copy Markdown

Purpose

This pull request should allow the Sphinx Python unit test suite to pass; it began failing recently following the release of Pygments v2.21.0 that included adjustments to HTML quote-escaping logic: pygments/pygments#3185

This changeset adapts the relevant unit test expectations according to the runtime version of Pygments detected.

References

AI Disclosure

I did not use any AI/LLM tooling for this pull request; neither to develop the changes locally, nor to create the pull request itself.


Edit: correction: a different Pygments PR seems to have been the cause of this; the hyperlink has been updated
Edit: rephrase for conciseness/clarity

1 similar comment
@saitanallensantiago26-beep

Copy link
Copy Markdown

Purpose

This pull request should allow the Sphinx Python unit test suite to pass; it began failing recently following the release of Pygments v2.21.0 that included adjustments to HTML quote-escaping logic: pygments/pygments#3185

This changeset adapts the relevant unit test expectations according to the runtime version of Pygments detected.

References

AI Disclosure

I did not use any AI/LLM tooling for this pull request; neither to develop the changes locally, nor to create the pull request itself.


Edit: correction: a different Pygments PR seems to have been the cause of this; the hyperlink has been updated
Edit: rephrase for conciseness/clarity

@Darlokt Darlokt mentioned this pull request Sep 19, 2026
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Oct 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: unit test failures due to HTML escaping change

7 participants